fix(chat): strip runtime turn context from projection before model input#560
Closed
sentry-junior[bot] wants to merge 2 commits into
Closed
fix(chat): strip runtime turn context from projection before model input#560sentry-junior[bot] wants to merge 2 commits into
sentry-junior[bot] wants to merge 2 commits into
Conversation
When a follow-up message arrives in an existing Slack thread, the session log projection may contain runtime turn context blocks that were written by prior turns (e.g. from the original thread starter). These blocks carry the earlier requester's identity. `loadPiMessagesForTurn` checked `hasRuntimeTurnContext` against the raw projection. If it returned true, the current turn's context (including the actual requester for this message) was never injected into the prompt. The model then used the stale requester identity for attributions such as the `Action taken on behalf of` line in PR bodies. The active-turn resume path already calls `stripRuntimeTurnContext` before returning piMessages. Apply the same strip to the projection path so both paths are consistent and fresh requester context is always injected for the current turn. Adds a regression test covering the multi-user thread scenario where the projection carries a stale requester block from user A, and user B's follow-up must inject fresh requester context. Refs: https://sentry.slack.com/archives/C0AHB7N2JCR/p1780911303272149 Co-authored-by: no <nelson.osacky@sentry.io>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-authored-by: no <nelson.osacky@sentry.io>
Member
|
Will dig into this. We likely need to maintain some identity block per message |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a follow-up message arrives in an existing Slack thread started by a different user, the requester attribution for model-generated content (such as the
Action taken on behalf ofline in PR bodies) would be wrong — showing the original thread starter instead of the person who actually made the request.Root Cause
The session log projection stores
pi_messageentries as-is, including any runtime turn context blocks (<runtime-turn-context>...<requester>...</requester>...) that were embedded in user messages from prior turns.In
loadPiMessagesForTurn, the active-turn resume path correctly strips old runtime context before returning:But the projection path did not strip:
When
hasRuntimeTurnContext(priorPiMessages)found the old context from user A's session, it returnedtrue, soneedsBootstrapContext = falseand user B's identity was never injected into the prompt. The model then used user A's stale<requester>block for any attributions it generated.Note: git commit co-author attribution was unaffected — that path uses the per-turn resolved identity directly rather than the model prompt.
Fix
Apply
stripRuntimeTurnContextto the projection path, making it consistent with the active-turn resume path:Context compaction already calls
stripRuntimeTurnContextinternally, so no double-strip concern.Verification
tsc --noEmitcleanrespond-helpers-runtime-context.test.ts)Remaining Risk
Other consumers of
loadProjectionthat checkhasRuntimeTurnContexton the result could have the same class of bug, though the main model-input path flows throughloadPiMessagesForTurn. Compaction is already safe (strips internally).